Conversation
…tch-up + T2-2 follow-up (Bundle b PR-3, Bb-3 順位 55) (#115) * `[review_recheck]` config セクションを `pr-monitor-config.toml` に追加し旧 hard-coded const (INITIAL_REVIEW_WAIT_SECS / REVIEW_RECHECK_WAIT_SECS / MAX_REVIEW_RECHECKS) を `PollContext` scalar 経由で thread。 * `MonitorConfig::poll_interval_secs` (Bb-2 で未使用化) を削除し、既存 toml は legacy フィールドを silently ignore で後方互換。 * `hooks-session-start` に PR monitor catch-up nudge を追加。state.action が `parked_*` かつ `next_wakeup_at_unix <= now` のとき additionalContext に `[PR_MONITOR_CATCHUP]` メッセージを注入し、CronCreate 失効時の silent loss を解消。 * terminal action (`action_required` / `passed_clean` / `continue_monitoring`) では古い park 由来の `next_wakeup_at_unix` 残存に対して false-positive nudge を action prefix gate で抑制。 * T2-2 follow-up (PR #114 post-merge-feedback): `finalize_park_siblings_have_symmetric_write_state_handling` を 3 sibling (rate_limit / review_recheck / initial_review) 化し、Bb-2 で追加した `finalize_initial_review_park` を parity invariant test の対象に含める (self-violation 解消)。 * 設計判断: SessionStart catch-up は別プロセス spawn せず additionalContext で nudge する設計 (advisor 推奨)。Windows 環境での handle 継承 / stdout 可視性問題を回避し、PARK signal flow を session 内に保つ。 ADR-030 の責務分離パターン (Rust 状態管理 + Claude periodic check) の 3 つ目の適用例として Bundle b 全体を完成させる。 由来: docs/coderabbit-monitoring-efficiency.md Bb-3 / 順位 55 / PR #114 post-merge-feedback T2-2 採用
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughハードコードされたレビュー再確認タイミング定数を新しい Changes設定リファクタリングとセッションスタート・キャッチアップ・ナッジ
Sequence DiagramsequenceDiagram
participant User as ユーザー
participant Hook as SessionStart Hook
participant FS as ファイルシステム
participant State as pr-monitor-state.json
participant Claude as Claude
User->>Hook: セッション開始イベント
Hook->>FS: セッションIDを書き込み（.session-id/.claude）
Hook->>FS: pr-monitor-state.json を読み取り
FS-->>Hook: 部分的な parked state
Hook->>Hook: action が "parked_*" か判定、next_wakeup_at_unix と現在時刻を比較
alt action が parked_* かつ next_wakeup_at_unix ≤ now
Hook->>Claude: additionalContext（CLAUDE_CODE_SESSION_ID） + キャッチアップナッジ（RESUME_MONITORING_COMMAND）
Claude-->>User: 再開ガイダンスを提示
else
Hook->>Claude: additionalContext のみを出力
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli-pr-monitor/src/config.rs`:
- Around line 129-159: ReviewRecheckConfig fields need range validation to avoid
zero or overflow: after deserializing the config in load_config, check
ReviewRecheckConfig.initial_review_wait_secs and review_recheck_wait_secs are >=
1 and <= i64::MAX (if out of range set them to
default_initial_review_wait_secs()/default_review_recheck_wait_secs()), and
ensure ReviewRecheckConfig.max_review_rechecks >= 1 (else set to
default_max_review_rechecks()); update load_config to perform these fallbacks so
later casts/Unix-timestamp math and the recheck-count logic don't break (refer
to ReviewRecheckConfig, initial_review_wait_secs, review_recheck_wait_secs,
max_review_rechecks, default_* functions).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3f37bfa-bc46-4f49-a161-4ac3f4808adc
📒 Files selected for processing (8)
docs/coderabbit-monitoring-efficiency.mddocs/todo.mddocs/todo5.mdpr-monitor-config.tomlsrc/cli-pr-monitor/src/config.rssrc/cli-pr-monitor/src/stages/poll.rssrc/cli-pr-monitor/src/state.rssrc/hooks-session-start/src/main.rs
💤 Files with no reviewable changes (1)
- docs/todo5.md
aloekun
added a commit
that referenced
this pull request
May 5, 2026
…ク (PR #115 CR Major #1 採用) * `max_review_rechecks=0` / `initial_review_wait_secs=0` / `review_recheck_wait_secs=0` / `wait_secs > i64::MAX` を defensive にデフォルト値 (3 / 300 / 300) にフォールバック。 * `load_config` 経路で `config.review_recheck = config.review_recheck.sanitize()` を適用。config は user 編集可能な system boundary のため、CLAUDE.md "ALWAYS validate at system boundaries" 原則に従う。 * sanitize 単体テスト 5 件追加 (zero max / zero waits / overflow / valid passthrough / i64::MAX boundary)。 由来: PR #115 CodeRabbit Major #1 採用 (id 3191214005)。
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli-pr-monitor/src/config.rs`:
- Around line 161-178: The addition now_unix + ctx.review_recheck_wait_secs as
i64 can overflow when review_recheck_wait_secs equals i64::MAX; update the logic
that sets state.next_wakeup_at_unix (in poll.rs where now_unix and
ctx.review_recheck_wait_secs are used) to perform a checked_add (e.g.,
now_unix.checked_add(wait as i64)) and handle the None case by capping to a safe
maximum (i64::MAX) or setting next_wakeup_at_unix to that max/None as
appropriate; alternatively enforce in ReviewRecheckConfig::sanitize that wait
values are <= (i64::MAX - now_unix) before casting, but the simplest fix is
replacing the direct + with checked_add and a clear fallback for overflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ade7ed65-7d14-4aa4-86b2-a6a2e606d5c7
📒 Files selected for processing (1)
src/cli-pr-monitor/src/config.rs
…ク (PR #115 CR Major #1 + #2 採用) * `max_review_rechecks=0` / `wait_secs=0` / `wait_secs > MAX_SAFE_WAIT_SECS (1 年)` を defensive にデフォルト値 (3 / 300 / 300) にフォールバック。 * `load_config` 経路で `config.review_recheck = config.review_recheck.sanitize()` を適用。config は user 編集可能な system boundary のため、CLAUDE.md "ALWAYS validate at system boundaries" 原則に従う。 * CR Major #2 (review id 3192783887) 採用: 上限を `i64::MAX` から `MAX_SAFE_WAIT_SECS = 365 * 24 * 60 * 60 = 31_536_000 (1 年)` に変更。`now_unix (~1.78e9 in 2026) + wait_secs as i64` の加算で確実に overflow しない範囲に制限。CronCreate auto-expire (7 日) より十分余裕。 * sanitize 単体テスト 5 件: zero max / zero waits / unrealistic (`u64::MAX` / `i64::MAX as u64`) / valid passthrough / `MAX_SAFE_WAIT_SECS` 境界値 + overflow invariant (`now_unix + sanitize 後の値` が `checked_add` で Some を返すこと) を machine-enforce。 由来: PR #115 CodeRabbit Major #1 (id 3191214005、resolved) + CodeRabbit Major #2 (id 3192783887)。
aloekun
force-pushed
the
feat/bb-3-config-and-catchup
branch
from
1cc0164 to
9e6bf1d
Compare
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Merged
5 tasks
aloekun
added a commit
that referenced
this pull request
May 6, 2026
…(PR #115 follow-up) (#116) * `coderabbit-monitoring-efficiency.md` Bb-3 エントリを「マージ済 (PR #115)」に更新、CR Major #1+#2 fold-in 修正の経緯と dogfood 実証 (Bundle b 全機能の連携) を記録。Bundle b 完結 (Bb-1 + Bb-2 + Bb-3 + 順位 75 + 76-78) を明記。 * `todo.md` priority table から完了済の順位 55 を削除し、PR #115 post-merge-feedback 採用 3 件を追加: 順位 76 (cross-module overflow 統合テスト) / 順位 77 (Unix timestamp baseline 境界値テスト) / 順位 78 (ADR-038 + CLAUDE.md security 拡充)。 * `todo5.md` に順位 76/77/78 の詳細セクション追加 (動機 / 設計決定 / 作業計画 / 完了基準 / 詰まっている箇所)。順位 76 と 77 は同一 PR で land 推奨 (両方 boundary を扱う test 層)、順位 78 は test 層 land 後の codification PR として位置づけ。 由来: PR #115 マージ + post-merge-feedback (`.claude/feedback-reports/115.md`、採用 3 / 様子見 4 / 却下 3)。
This was referenced May 6, 2026
aloekun
added a commit
that referenced
this pull request
May 7, 2026
…位 76+77 / §A-2 P-4) (#128) * test(cli-pr-monitor): cross-module overflow safety + 2100 baseline + §A-2 P-3 ledger (順位 76+77 / §A-2 P-4) §A-2 Phase 5 dogfood P-4 (Bb-3 follow-up)。PR #115 で実証された unit test isolation 起因の cross-module overflow 自己矛盾 (config layer の sanitize テストが downstream poll arithmetic の overflow safety を verify していなかった) を統合テストで machine-enforce。 順位 76 (Tier 2, T2-1): - 4 cross-module integration tests を config.rs test module に追加 - cross_module_overflow_safety_at_max_boundary (sanitize 後の MAX_SAFE_WAIT_SECS が finalize_initial_review_park / schedule_next_* 両経路で overflow しないことを assert) - cross_module_overflow_safety_with_zero_input_uses_default (0 → default 300s) - cross_module_overflow_safety_with_u64_max_input_uses_default (u64::MAX → default 300s) - cross_module_overflow_safety_negative_test_large_unsanitized_value_overflows (i64::MAX as u64 を直接 cast すると positive overflow、sanitize の必要性裏付け) - cross_module_overflow_safety_negative_test_u64_max_wraps_to_minus_one (u64::MAX as i64 = -1 で silent corruption (過去時刻)、sanitize で防止) 順位 77 (Tier 2, T2-2): - 既存 review_recheck_sanitize_max_safe_boundary テストを year 2100 baseline (now_unix = 4.1e9) でも overflow しないことを assert する形に拡張 - MAX_SAFE_WAIT_SECS 定数 doc に future-proof 根拠を追記 (year 2100 baseline で safety margin が 9 桁残ることを明文化) §A-2 計測ログ P-3 結果記録: - P-3 (PR #127): rate-limit blocked (CR が 27 min wait の rate-limit overlay で formal review 投稿不可)、classifier 未起動、計測 N/A — dogfood 不発。但し CR rate-limit 経路が dogfood の阻害要因として観測 (§A-2 §6 注意事項 #1 を実証)。 テスト 168 active passed (順位 76 系 5 件追加)、regression なし。 deployed exe には影響なし (test only + doc コメント)。 * docs(todo): 順位 76 + 77 (cross-module overflow + 境界値 matrix) 完了に伴い削除
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundle b の最終 PR (Bb-3 / 順位 55)。Bb-1 (PR #113) / Bb-2 (PR #114) で導入した CronCreate park モデルの後始末として、固定値の config 化 + SessionStart catch-up + T2-2 follow-up を 1 PR で集約 land する。これで Bundle b 全体が完結。
ADR-030 の責務分離パターン (Rust 状態管理 + Claude Code 側の周期確認) の 3 つ目の適用例 として、Bundle b 全体が「Rust が状態を持ち、Claude Code が CronCreate / SessionStart 経由で再開する」モデルに統一される。
変更内容
1. config 拡張 — 旧 hard-coded const →
[review_recheck]セクションpr-monitor-config.tomlに新セクション[review_recheck]追加 (initial_review_wait_secs/review_recheck_wait_secs/max_review_rechecks、デフォルト 300/300/3)INITIAL_REVIEW_WAIT_SECS/REVIEW_RECHECK_WAIT_SECS/MAX_REVIEW_RECHECKS) をPollContext経由の scalar threading で置換MonitorConfig::poll_interval_secs(Bb-2 で#[allow(dead_code)]化済) を完全削除poll_interval_secs行は serde が silently ignore (config_ignores_legacy_poll_interval_secsテストで machine-enforce)2. SessionStart catch-up nudge —
additionalContext経由 (advisor 推奨 option b)hooks-session-startが起動時にpr-monitor-state.jsonを partial deserializestate.action.starts_with("parked_")かつnext_wakeup_at_unix <= nowのときadditionalContextに[PR_MONITOR_CATCHUP]メッセージを注入RESUME_MONITORING_COMMANDconst に切り出し (rename 時の drift 検知をテストで machine-enforce)cli-pr-monitor.exe --monitor-only) ではなく Claude に nudge する設計を採用。Windows 環境での handle 継承 / stdout 可視性問題を回避し、PARK signal flow を session 内に保つaction_required/passed_clean/continue_monitoring) では古い park 由来のnext_wakeup_at_unix残存に対して nudge を出さない (action prefixparked_を gate に使用)3. T2-2 follow-up — sibling parity invariant の 3 sibling 化
PR #114 post-merge-feedback Tier 2 #2 採用分。
finalize_park_siblings_have_symmetric_write_state_handlingテストのカバレッジにfinalize_initial_review_parkを追加し、Bb-2 同時追加の sibling が parity invariant test の対象外になる self-violation を解消。invoke_finalize_initial_review_park_with_bad_pathtest helper を追加し、既存invoke_finalize_parked_with_bad_path/invoke_review_park_with_bad_pathと並べて 3 sibling all-pass を assert。4. 補助テスト
format_review_park_signal_uses_configured_max_rechecks— config 値 (例: 7) が PARK signal 出力に到達することを machine-enforce (default 3 が hard-coded で残っていないことを assert)compute_catchup_nudgeの decision matrix (wakeup なし / 未来 / 過去 / terminal action 4 種 + parked 2 種) を 8 unit test でカバー設計参照
docs/coderabbit-monitoring-efficiency.mdBb-3 エントリ.claude/feedback-reports/114.md)pre-push-review 結果
all("approved")(1 iteration、6m 54s)。レビュアーからRESUME_MONITORING_COMMANDconst 抽出を有効なリグレッション保護として positive 言及あり (OBS-2)。Test Plan
cargo test -p cli-pr-monitor(137 件 全 pass、parity test 3 sibling 化、PARK signal config 反映 test を含む)cargo test -p hooks-session-start(22 件 全 pass、catch-up nudge 8 軸テスト + RESUME_MONITORING_COMMAND assertion)cargo clippy -p cli-pr-monitor -p hooks-session-start --all-targets -- -D warnings(warning ゼロ)cargo fmt --all(workspace 全体)pnpm build:all成功all("approved")pr-monitor-config.tomlから legacypoll_interval_secs行を削除 (silently ignore で実害なしだが clean up)Summary by CodeRabbit
新機能
ドキュメント
Chores
Tests